-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[App Service] az functionapp create
: Create consumption plan when creating the function app
#30612
base: dev
Are you sure you want to change the base?
Conversation
️✔️AzureCLI-FullTest
|
Hi @kamperiadis, |
️✔️AzureCLI-BreakingChangeTest
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
@zhoxing-ms It seems the test failures are just happening because the name does not match
Is there any way for this name validation to be ignored? Or do I just need to convert these tests to be run live instead? |
305c42c
to
f93258b
Compare
f93258b
to
900d762
Compare
The tests in this PR fail for the same reason this other PR fails: #30605 (comment) Should we make these tests run in live mode instead? |
@@ -4922,6 +4922,11 @@ def create_functionapp(cmd, resource_group_name, name, storage_account, plan=Non | |||
functionapp_def.kind = 'functionapp' | |||
# if os_type is None, the os type is windows | |||
is_linux = bool(os_type and os_type.lower() == LINUX_OS_NAME) | |||
plan_name = generatePlanName(resource_group_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the reason why the recording tests failed. A new random plan name will be generated when each time this function is run. Thus, when tests are run under recording mode, the plan name is generated again and different from what in the recoding file.
azure-cli/src/azure-cli/azure/cli/command_modules/appservice/custom.py
Lines 5394 to 5398 in 900d762
def generatePlanName(resource_group_name): | |
suffix = "-" + str(uuid.uuid4())[:4] | |
alphanumeric_resource_group_name = re.sub(r"[^a-zA-Z0-9]", '', resource_group_name) | |
first_part = 'ASP-{}'.format(alphanumeric_resource_group_name)[:35] | |
return '{}{}'.format(first_part, suffix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @yanzhudd We actually do intend to create a new plan resource with a new name on behalf of the customer. Does this mean we should just make the tests run live? Or what would you require for this PR to be merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, please mark the related tests as live only, and remove the corresponding recording files as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Thank you @yanzhudd. The PR is ready to be merged. Can you please review/merge it?
az functionapp create
: Create consumption plan when creating the function appaz functionapp create
: Create consumption plan when creating the function app
ca01570
to
6b0a7d9
Compare
Related command
az functionapp create
Description
Resolves #28944
This issue is happening because we are currently relying on the backend to create the consumption plan but instead, the Az CLI should be creating the plan and then passing that in the payload to the backend.
Testing Guide
az functionapp create -c <location> -n <name> -g <resourceGroupName> --runtime <runtime> -s <storageName> --functions-version 4
History Notes
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.